[eas-cli] Add --refresh-distribution-certificate for non-intractive builds #3739
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
--refresh-distribution-certificate for non-intractive builds
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## szymonswierk/non-interactive-build-provisioning-profile-validationa-and-refresh-best-effort #3739 +/- ##
===============================================================================================================================
+ Coverage 57.86% 57.99% +0.13%
===============================================================================================================================
Files 913 913
Lines 39622 39646 +24
Branches 8296 8311 +15
===============================================================================================================================
+ Hits 22925 22987 +62
+ Misses 15242 15207 -35
+ Partials 1455 1452 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Subscribed to pull request
Generated by CodeMention |
763d237 to
1e21106
Compare
701b7d7 to
4c939d9
Compare
354509c to
e1e9a7e
Compare
quinlanj
left a comment
There was a problem hiding this comment.
lgtm. I'm surprised this was a feature people asked for because the number of dist certs apple lets you have in your account is very low (2-3 max)
d20291d to
d4b4bf6
Compare
e1e9a7e to
6436e27
Compare
| throw new Error( | ||
| 'No App Store Connect API Key found for distribution certificate refresh. In non-interactive mode, provide one via:\n' + | ||
| ' - Environment variables: EXPO_ASC_API_KEY_PATH, EXPO_ASC_KEY_ID, EXPO_ASC_ISSUER_ID\n' + | ||
| ' - EAS credentials service: configure an App Store Connect API Key for submissions on this app' |
There was a problem hiding this comment.
ugh it's still weird to me we're saying to use key for submissions to generate certificates
| Log.log(`Reusing distribution certificate with serial number ${cert.serialNumber}`); | ||
| return cert; | ||
| } | ||
| return await this.createNewDistCertAsync(ctx); |
There was a problem hiding this comment.
how likely is that we're going to hit
often? can we do something about it?There was a problem hiding this comment.
We're going to hit this when a new certificate needs to be created and there are already 3 (non-matching, e.g. for different teams) certificates in the Apple account. I don't think we should do anything automatically in this case, as this would involve revoking unrelated certs. We could make an opt-in flag to "force override" a cert.
6436e27 to
424bd35
Compare
…fort Remove the --refresh-distribution-certificate opt-in flag and validate or refresh distribution certificates automatically in non-interactive builds, reusing tryAuthenticateAppStoreWithEasAscApiKeyAsync from the parent stack.
424bd35 to
0d88201
Compare
15311b5 to
dc5b262
Compare
--refresh-distribution-certificate for non-intractive builds|
Changed this to follow the "best effort" approach like provisioning profiles in #3805 instead of being opt-in with a flag, as I don't see any downsides of just attempting to do the validation when an ASC API key is available in EAS. If we were to do something like "force setup dist cert even if some other unrelated (other apple team) cert needs to be revoked", that would be an opt-in flag for special use cases. @quinlanj Apologies, re-requesting review. |
quinlanj
left a comment
There was a problem hiding this comment.
The code looks beautiful, I'd push back on this from a product perspective though. For a lot of developers, distribution certs are often treated like account-wide signing infrastructure, rather than app-local disposable state. I suspect there will be users who will be unpleasantly surprised to discover the submission API key was used as implicit consent to create signing certs in non interactive mode.
My vote is to put this behind the original flag you had unless you felt strongly about this
|
✅ Thank you for adding the changelog entry! |
This is some amazing feedback, thank you. Reverting this to the opt-in approach, I'm happy to lean on your expertise |

Why
When there's no valid distribution certificate associated with an app, and a non-interactive development build is run (e.g. invoked by CI or by a workflow job), even without the freeze-credentials flag, the distribution certificate is not validated or refreshed, which may fail the internal build and require doing manual
eas buildto create a new certificate. See the Linear issue: https://linear.app/expo/issue/ENG-21330/make-it-possible-to-refresh-a-distribution-certificate-in-nonHow
Added an opt-in distribution certificate refresh to non-interactive builds with flag
--refresh-distribution-certificate.When the flag is present, we hit www GQL API to get the ASC API key (the submission key), and we distribution certificate validation or set-up if needed.
packages/eas-cli/src/credentials/ios/actions/SetUpDistributionCertificate.tsis where this logic happens.The flag mustn't be present when
--freeze-credentialsis present, these are conflicting flags.I'm leaning towards an opt-in flag instead of extending the default behavior because of the extra credentials configuration requirements and side effects of the distribution certificate refresh logic.
Test Plan
Added unit tests.
Manual verification:
easd build --platform ios --profile development --non-interactive --refresh-distribution-certificatein the following scenarios, verified it works:easd build --platform ios --profile development --non-interactive --refresh-distribution-certificate --refresh-ad-hoc-provisioning-profile, verified the ASC API key got resolved just once.